Skip to content
This repository was archived by the owner on Mar 1, 2024. It is now read-only.

Added tests for vm list/detail vm view actions (start, stop, restart, delete)#163

Merged
atiratree merged 1 commit intokubevirt:masterfrom
rhrazdil:vm_list_view_actions
Jan 17, 2019
Merged

Added tests for vm list/detail vm view actions (start, stop, restart, delete)#163
atiratree merged 1 commit intokubevirt:masterfrom
rhrazdil:vm_list_view_actions

Conversation

@rhrazdil
Copy link

@rhrazdil rhrazdil commented Jan 8, 2019 •
edited
Loading

https://cnv-qe-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/test-ocp-3.11-cnv-1.4-integration-web-ui-rhrazdil/30/console

question:
I'm not quite satisfied with defining the VM object in scenario.ts file, especially when is this long. @vojtechszocs What do you think about it? Maybe we could create some resource file wher CRDs could be placed.

@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch 3 times, most recently from 208ce97 to 97fc678 Compare January 9, 2019 12:29
@rhrazdil rhrazdil changed the title [WIP] Added tests for vm list/vies actions (start, stop, restart, delete) Added tests for vm list/vies actions (start, stop, restart, delete) Jan 9, 2019
@rhrazdil rhrazdil changed the title Added tests for vm list/vies actions (start, stop, restart, delete) Added tests for vm list/views actions (start, stop, restart, delete) Jan 9, 2019
@rhrazdil rhrazdil changed the title Added tests for vm list/views actions (start, stop, restart, delete) Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 9, 2019
@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch from 97fc678 to 544367d Compare January 9, 2019 12:43
atiratree
atiratree suggested changes Jan 9, 2019
View reviewed changes
frontend/integration-tests/views/kubevirt/vm.actions.view.ts Outdated
Copy link

@atiratree atiratree Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to extract common functionality from listViewAction and detailViewAction? And also, extracting the kebap into a variable?

Copy link
Author

@rhrazdil rhrazdil Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Couldn't find a way to extract the kebab element, for some reason when I had
export const $('.co-kebab__dropdown');
before the action function, the element was undefined.

Copy link

@atiratree atiratree Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coulnd't we just pass a function, something like getActionsDropdown()?

Copy link
Author

@rhrazdil rhrazdil Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, we can't. Such function would have to be implemented by ElementFinder, which is what rowForName(name) returns.
But maybe just defining the string could be OK?
Like:
const listKebabDropdown = '.co-kebab__dropdown';
rowForName(name).$(listKebabDropdown)...

Copy link

@atiratree atiratree Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this work or am I missing something?

listViewAction = (name) => {
    getActionsDropdown = () => rowForName(name).$$('.co-kebab__button').first();
    getActionsDropdownMenu = () => rowForName(name).$(listViewKebabDropdown);
    return selectDropdownItem(getActionsDropdown, getActionsDropdownMenu);
}

selectDropdownItem  = (getActionsDropdown, getActionsDropdownMenu) => (action) =>
.
. common functionality (calls getActionsDropdown and getActionsDropdownMenu)
.

and detailViewAction implemented with the same mechanism

Copy link
Author

@rhrazdil rhrazdil Jan 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I'm volunteering on QE Camp. Yes, this approach will work.

Copy link
Author

@rhrazdil rhrazdil Jan 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@vojtechszocs
Copy link

vojtechszocs commented Jan 9, 2019

question:
I'm not quite satisfied with defining the VM object in scenario.ts file, especially when is this long. @vojtechszocs What do you think about it? Maybe we could create some resource file wher CRDs could be placed.

I'd suggest to have simple tests along with reusable utilities.

As for *.scenario.ts files:

  • each describe represents a logical test suite, this is useful for organizing test code
  • each it (or test) represents a specific test case => specific UI workflow
  • try to keep test cases focused => end up having multiple test cases => easier to fix or update
  • after each test suite, the state of the k8s system should be like before that test suite

As for utilities:

  • I'd suggest adding e.g. frontend/integration-tests/tests/kubevirt/vm-utils.ts and use kubectl for programmatic k8s object management
  • templates for test-related k8s objects can be placed in e.g. frontend/integration-tests/tests/kubevirt/vm-mocks.ts
  • this helps to keep our tests focused on actual testing, e.g. for a test that operates on existing VMs, it's not really important how those VMs are created
  • this doesn't impact tests that exercise UI workflows like create VM via wizard or YAML

Following the existing convention, every created k8s object (regardless if created via UI workflow or via kubectl) should have a label automatedTestName: <testName> where <testName> can be taken as-is, or computed, from integration-tests/protractor.conf.ts.

@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch from 544367d to 3d0ebaa Compare January 10, 2019 12:51
@vojtechszocs vojtechszocs mentioned this pull request Jan 11, 2019
Closed
3 tasks
@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch 4 times, most recently from abdaaf9 to ff15532 Compare January 14, 2019 14:00
@rhrazdil
Copy link
Author

rhrazdil commented Jan 14, 2019

I have refactored the VM provision tests, now there are 3 specs for each provision source.
Common functionality is being moved to utils.ts and CRDs to mocks.ts (I deviated from vm-* as it's already more general, for example there is network attachment definition in mocks.ts).

Comments regarding vm actions are fixed as well and ready for review.

@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch 2 times, most recently from 5466b69 to 8d7b026 Compare January 14, 2019 15:48
@rhrazdil rhrazdil changed the title Added tests for vm list/detail vm view actions (start, stop, restart, delete) [WIP] Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 14, 2019
@rhrazdil
Copy link
Author

rhrazdil commented Jan 14, 2019

Added WIP label again because there seems to be a issue with clicking the dropdown button. It's like the dropdown closes right after it is opened.

@rhrazdil rhrazdil mentioned this pull request Jan 16, 2019
Closed
@rhrazdil rhrazdil force-pushed the vm_list_view_actions branch from 8d7b026 to 9c440a2 Compare January 16, 2019 16:32
@rhrazdil rhrazdil changed the title [WIP] Added tests for vm list/detail vm view actions (start, stop, restart, delete) Added tests for vm list/detail vm view actions (start, stop, restart, delete) Jan 17, 2019
@atiratree atiratree merged commit 4a3c423 into kubevirt:master Jan 17, 2019
@vojtechszocs
Copy link

vojtechszocs commented Jan 18, 2019

Thanks a lot @rhrazdil, I'm going to backport those changes into openshift#888.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@atiratree atiratree atiratree approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rhrazdil @vojtechszocs @atiratree @kubevirt-bot